Skip to content

Conversation

@justincady
Copy link
Contributor

Calls to noreturn functions result in region termination for
coverage mapping. But this creates incorrect coverage results when
noreturn functions (or other constructs that result in region
termination) occur within GNU statement expressions.

In this scenario an extra gap region is introduced within VisitStmt,
such that if the following line does not introduce a new region it
is unconditionally counted as uncovered.

This change adjusts the mapping such that terminate statements
within statement expressions do not propagate that termination
state after the statement expression is processed.

Fixes #124296

@llvmbot llvmbot added clang Clang issues not falling into any other category compiler-rt clang:codegen IR generation bugs: mangling, exceptions, etc. PGO Profile Guided Optimizations labels Mar 12, 2025
@justincady justincady requested a review from ZequanWu March 12, 2025 15:25
@llvmbot
Copy link
Member

llvmbot commented Mar 12, 2025

@llvm/pr-subscribers-clang-codegen
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-pgo

Author: Justin Cady (justincady)

Changes

Calls to noreturn functions result in region termination for
coverage mapping. But this creates incorrect coverage results when
noreturn functions (or other constructs that result in region
termination) occur within GNU statement expressions.

In this scenario an extra gap region is introduced within VisitStmt,
such that if the following line does not introduce a new region it
is unconditionally counted as uncovered.

This change adjusts the mapping such that terminate statements
within statement expressions do not propagate that termination
state after the statement expression is processed.

Fixes #124296


Full diff: https://github.com/llvm/llvm-project/pull/130976.diff

3 Files Affected:

  • (modified) clang/lib/CodeGen/CoverageMappingGen.cpp (+8)
  • (modified) clang/test/CoverageMapping/terminate-statements.cpp (+7)
  • (added) compiler-rt/test/profile/Linux/coverage-statement-expression.cpp (+19)
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index f09157771d2b5..73811d15979d5 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1505,6 +1505,14 @@ struct CounterCoverageMappingBuilder
     handleFileExit(getEnd(S));
   }
 
+  void VisitStmtExpr(const StmtExpr *E) {
+    Visit(E->getSubStmt());
+    // Any region termination (such as a noreturn CallExpr) within the statement
+    // expression has been handled by visiting the sub-statement. The visitor
+    // cannot be at a terminate statement leaving the statement expression.
+    HasTerminateStmt = false;
+  }
+
   void VisitDecl(const Decl *D) {
     Stmt *Body = D->getBody();
 
diff --git a/clang/test/CoverageMapping/terminate-statements.cpp b/clang/test/CoverageMapping/terminate-statements.cpp
index 0067185fee8e6..d03e35630b317 100644
--- a/clang/test/CoverageMapping/terminate-statements.cpp
+++ b/clang/test/CoverageMapping/terminate-statements.cpp
@@ -346,6 +346,12 @@ int elsecondnoret(void) {
   return 0;
 }
 
+// CHECK-LABEL: _Z18statementexprnoretb
+int statementexprnoret(bool crash) {
+  int rc = ({ if (crash) abort(); 0; }); // CHECK-NOT: Gap,File 0, [[@LINE]]:41 -> [[@LINE+1]]:3 = 0
+  return rc;
+}
+
 int main() {
   foo(0);
   foo(1);
@@ -368,5 +374,6 @@ int main() {
   ornoret();
   abstractcondnoret();
   elsecondnoret();
+  statementexprnoret(false);
   return 0;
 }
diff --git a/compiler-rt/test/profile/Linux/coverage-statement-expression.cpp b/compiler-rt/test/profile/Linux/coverage-statement-expression.cpp
new file mode 100644
index 0000000000000..5894275b941a2
--- /dev/null
+++ b/compiler-rt/test/profile/Linux/coverage-statement-expression.cpp
@@ -0,0 +1,19 @@
+// RUN: %clangxx_profgen -std=gnu++17 -fuse-ld=lld -fcoverage-mapping -o %t %s
+// RUN: env LLVM_PROFILE_FILE=%t.profraw %run %t
+// RUN: llvm-profdata merge -o %t.profdata %t.profraw
+// RUN: llvm-cov show %t -instr-profile=%t.profdata 2>&1 | FileCheck %s
+
+#include <stdio.h>
+
+__attribute__ ((__noreturn__))
+void foo(void) { while (1); }                   // CHECK:  [[@LINE]]| 0|void foo(void)
+_Noreturn void bar(void) { while (1); }         // CHECK:  [[@LINE]]| 0|_Noreturn void bar(void)
+                                                // CHECK:  [[@LINE]]|  |
+int main(int argc, char **argv) {               // CHECK:  [[@LINE]]| 1|int main(
+  int rc = ({ if (argc > 3) foo(); 0; });       // CHECK:  [[@LINE]]| 1|  int rc =
+  printf("coverage after foo is present\n");    // CHECK:  [[@LINE]]| 1|  printf(
+                                                // CHECK:  [[@LINE]]|  |
+  int rc2 = ({ if (argc > 3) bar(); 0; });      // CHECK:  [[@LINE]]| 1|  int rc2 =
+  printf("coverage after bar is present\n");    // CHECK:  [[@LINE]]| 1|  printf(
+  return rc + rc2;                              // CHECK:  [[@LINE]]| 1|  return rc
+}                                               // CHECK:  [[@LINE]]| 1|}

@justincady justincady requested a review from chapuni March 12, 2025 15:26
@github-actions
Copy link

github-actions bot commented Mar 12, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@justincady justincady requested a review from MaskRay March 13, 2025 13:36
Calls to __noreturn__ functions result in region termination for
coverage mapping. But this creates incorrect coverage results when
__noreturn__ functions (or other constructs that result in region
termination) occur within [GNU statement expressions][1].

In this scenario an extra gap region is introduced within VisitStmt,
such that if the following line does not introduce a new region it
is unconditionally counted as uncovered.

This change adjusts the mapping such that terminate statements
within statement expressions do not propagate that termination
state after the statement expression is processed.

[1]: https://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html

Fixes #124296
@justincady justincady merged commit 0827e3a into llvm:main Mar 19, 2025
6 of 10 checks passed
@justincady justincady deleted the stmt-expr-coverage branch March 19, 2025 20:23
@llvm-ci
Copy link
Collaborator

llvm-ci commented Mar 19, 2025

LLVM Buildbot has detected a new failure on builder clang-cmake-x86_64-avx512-linux running on avx512-intel64 while building clang,compiler-rt at step 7 "ninja check 1".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/133/builds/13131

Here is the relevant piece of the build log for the reference
Step 7 (ninja check 1) failure: stage 1 checked (failure)
******************** TEST 'Profile-i386 :: Linux/coverage-statement-expression.cpp' FAILED ********************
Exit Code: 1

Command Output (stderr):
--
RUN: at line 1: /localdisk2/buildbot/llvm-worker/clang-cmake-x86_64-avx512-linux/stage1/./bin/clang  --driver-mode=g++  -m32  -ldl  -fprofile-instr-generate -std=gnu++17 -fuse-ld=lld -fcoverage-mapping -o /localdisk2/buildbot/llvm-worker/clang-cmake-x86_64-avx512-linux/stage1/runtimes/runtimes-bins/compiler-rt/test/profile/Profile-i386/Linux/Output/coverage-statement-expression.cpp.tmp /localdisk2/buildbot/llvm-worker/clang-cmake-x86_64-avx512-linux/llvm/compiler-rt/test/profile/Linux/coverage-statement-expression.cpp
+ /localdisk2/buildbot/llvm-worker/clang-cmake-x86_64-avx512-linux/stage1/./bin/clang --driver-mode=g++ -m32 -ldl -fprofile-instr-generate -std=gnu++17 -fuse-ld=lld -fcoverage-mapping -o /localdisk2/buildbot/llvm-worker/clang-cmake-x86_64-avx512-linux/stage1/runtimes/runtimes-bins/compiler-rt/test/profile/Profile-i386/Linux/Output/coverage-statement-expression.cpp.tmp /localdisk2/buildbot/llvm-worker/clang-cmake-x86_64-avx512-linux/llvm/compiler-rt/test/profile/Linux/coverage-statement-expression.cpp
clang: error: invalid linker name in argument '-fuse-ld=lld'

--

********************


justincady added a commit that referenced this pull request Mar 19, 2025
…ns" (#132095)

Reverts #130976

Breaks clang-cmake-x86_64-avx512-linux bot.
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Mar 19, 2025
…t expressions" (#132095)

Reverts llvm/llvm-project#130976

Breaks clang-cmake-x86_64-avx512-linux bot.
justincady added a commit to justincady/llvm-project that referenced this pull request Mar 20, 2025
Relands llvm#130976 with adjustments to test requirements.

Calls to __noreturn__ functions result in region termination for
coverage mapping. But this creates incorrect coverage results when
__noreturn__ functions (or other constructs that result in region
termination) occur within [GNU statement expressions][1].

In this scenario an extra gap region is introduced within VisitStmt,
such that if the following line does not introduce a new region it
is unconditionally counted as uncovered.

This change adjusts the mapping such that terminate statements
within statement expressions do not propagate that termination
state after the statement expression is processed.

[1]: https://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html

Fixes llvm#124296
justincady added a commit to justincady/llvm-project that referenced this pull request Mar 21, 2025
Relands llvm#130976 with adjustments to test requirements.

Calls to __noreturn__ functions result in region termination for
coverage mapping. But this creates incorrect coverage results when
__noreturn__ functions (or other constructs that result in region
termination) occur within [GNU statement expressions][1].

In this scenario an extra gap region is introduced within VisitStmt,
such that if the following line does not introduce a new region it
is unconditionally counted as uncovered.

This change adjusts the mapping such that terminate statements
within statement expressions do not propagate that termination
state after the statement expression is processed.

[1]: https://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html

Fixes llvm#124296
justincady added a commit that referenced this pull request Mar 21, 2025
#132222)

Relands #130976 with adjustments to test requirements.

Calls to __noreturn__ functions result in region termination for
coverage mapping. But this creates incorrect coverage results when
__noreturn__ functions (or other constructs that result in region
termination) occur within [GNU statement expressions][1].

In this scenario an extra gap region is introduced within VisitStmt,
such that if the following line does not introduce a new region it
is unconditionally counted as uncovered.

This change adjusts the mapping such that terminate statements
within statement expressions do not propagate that termination
state after the statement expression is processed.

[1]: https://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html

Fixes #124296
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:codegen IR generation bugs: mangling, exceptions, etc. clang Clang issues not falling into any other category compiler-rt PGO Profile Guided Optimizations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[coverage] __noreturn__ attribute leads to incorrect coverage report

4 participants